chore: replace ory/dockertest with testcontainers by miparnisari · Pull Request #2782 · authzed/spicedb

miparnisari · 2025-12-18T20:01:26Z

No description provided.

codecov · 2025-12-18T20:04:31Z

Codecov Report

❌ Patch coverage is 90.37433% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.25%. Comparing base (3fa88f3) to head (1495951).
⚠️ Report is 16 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/testserver/datastore/spanner.go	80.00%	6 Missing and 1 partial ⚠️
internal/testserver/datastore/mysql.go	85.72%	4 Missing and 1 partial ⚠️
internal/testserver/datastore/crdb.go	93.62%	2 Missing and 1 partial ⚠️
internal/testserver/datastore/postgres.go	95.72%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2782      +/-   ##
==========================================
- Coverage   77.72%   75.25%   -2.47%     
==========================================
  Files         472      472              
  Lines       49707    49727      +20     
==========================================
- Hits        38631    37416    -1215     
- Misses       8228     9494    +1266     
+ Partials     2848     2817      -31

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mdelapenya

This looks really promising! I added some comments regarding best practices when using the library. Feel free to ping me for anything you need, I'm more than happy to help you with this 😊

mdelapenya · 2026-01-29T10:22:25Z

cmd/spicedb/integration/migrate_integration_test.go

 	require.NoError(t, err)
 	t.Cleanup(func() {
-		_ = pool.Client.RemoveNetwork(network.ID)
+		_ = net.Remove(ctx)


suggestion: use CleanupNetwork, and call it before require, as it handle nils

nw, err := network.New(ctx, network.WithDriver("bridge"), network.WithLabels(map[string]string{"name": bridgeNetworkName})) testcontainers.CleanupNetwork(t, nw) require.NoError(t, err)

mdelapenya · 2026-01-29T10:23:47Z

cmd/spicedb/integration/migrate_integration_test.go

-				config.RestartPolicy = docker.RestartPolicy{
-					Name: "no",
-				}
+			container, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{


question: why not using the module? https://github.com/Mariscal6/testcontainers-spicedb-go

If it does not provide the APIs need by these tests, we can contribute them to the upstream. I'm pretty sure @Mariscal6 will be happy to review that

mdelapenya · 2026-01-29T10:26:54Z

cmd/spicedb/integration/migrate_integration_test.go

 			require.NoError(t, err)
 			t.Cleanup(func() {
-				_ = pool.Purge(resource)
+				_ = container.Terminate(ctx)
 			})


suggestion: use CleanupContainer, as for networks.

ctr, err := ... // module's Run function OR the GenericContainer() one. testcontainers.CleanupContainer(t, ctr) require.NoError(t, err)

mdelapenya · 2026-01-29T10:28:23Z

cmd/spicedb/integration/migrate_integration_test.go

 			require.NoError(t, err)

-			if status != 0 {
+			if state.ExitCode != 0 {


suggestion: you could define a log consumer at container creation, and print it here. Please see https://golang.testcontainers.org/features/follow_logs/#following-container-logs

mdelapenya · 2026-01-29T10:30:49Z

cmd/spicedb/integration/schemawatch_integration_test.go

-				config.RestartPolicy = docker.RestartPolicy{
-					Name: "no",
-				}
+			migrateContainer, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{


suggestion: given there are a few of this, you could abstract the container creation building your own test "framework", something like this:

// pseudocode func NewTestSpiceDB(ctx Context, opts ...ContainerOpts) {...}

mdelapenya · 2026-01-29T10:55:54Z

internal/testserver/datastore/postgres.go

 }

-func createNetworkBridge(t testing.TB, pool *dockertest.Pool) string {
+func createNetworkBridge(t testing.TB) (string, *testcontainers.DockerNetwork) {


suggestion: you could expose this in the "framework" and use it everywhere else that you create a network. Adding the CleanupNetwork here would automatically clean up resources with every call.

mdelapenya · 2026-01-29T10:57:02Z

internal/testserver/datastore/postgres.go

-			"MAX_CLIENT_CONN=" + PostgresTestMaxConnections,
-		},
+	req := testcontainers.ContainerRequest{
+		Name:         pgbouncerContainerHostname,


suggestion: remember the advice against names :)

mdelapenya · 2026-01-29T10:58:42Z

internal/testserver/datastore/postgres.go

+			WithStartupTimeout(dockerBootTimeout),
+	}
+
+	if bridgeNetworkName != "" {


suggestion: if moving to the Run function with container customisers as functional options, then you could be passing any of the WithNetworkXXX options. See https://golang.testcontainers.org/features/creating_container/#networking-options

mdelapenya · 2026-01-29T10:59:44Z

internal/testserver/datastore/postgres.go

-		},
+	req := testcontainers.ContainerRequest{
+		Name:         pgbouncerContainerHostname,
+		Image:        "mirror.gcr.io/edoburu/pgbouncer:latest",


question: is this a postgres db instance? Try using the Postgres module with your pgBouncer image, so you get the Postgres module's APIs. Then you could remove all the wait code below

mdelapenya · 2026-01-29T11:00:47Z

internal/testserver/datastore/spanner.go

 	name := "spanner-" + uuid.New().String()
-	resource, err := pool.RunWithOptions(&dockertest.RunOptions{
+
+	req := testcontainers.ContainerRequest{


suggestion: we have a module for Spanner :)

Yeah, the issue we've found is that the spanner emulator doesn't replicate the behaviors that we need faithfully enough to be of much use.

Oh that's good to know! Is this something we can add to the upstream module?

github-actions bot added area/cli Affects the command line area/datastore Affects the storage system area/dependencies Affects dependencies area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Dec 18, 2025

miparnisari changed the title ~~chore: use testcontainers~~ chore: replace ory/dockertest with testcontainers Jan 28, 2026

mdelapenya reviewed Jan 29, 2026

View reviewed changes

miparnisari and others added 4 commits March 3, 2026 13:53

chore: use testcontainers

9bf1c74

chore: do fixups

fbe233e

chore: use testcontainers for crdb

86b9554

chore: try using postgres testcontainer to wrap pgbouncer

100110f

tstirrat15 force-pushed the use-testcontainers branch from 1495951 to 100110f Compare March 3, 2026 20:57

Conversation

miparnisari commented Dec 18, 2025

Uh oh!

codecov bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mdelapenya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Dec 18, 2025 •

edited

Loading